-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(models): Expose endpoint in project key model serializer #102083
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| "playstation": obj.playstation_endpoint, | ||
| "otlp_traces": obj.otlp_traces_endpoint, | ||
| "otlp_logs": obj.otlp_logs_endpoint, | ||
| "endpoint": obj.get_endpoint(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Serializing multiple ProjectKey objects triggers N+1 database queries to fetch project.organization due to missing select_related.
Severity: HIGH | Confidence: 0.95
🔍 Detailed Analysis
When the API endpoint /api/0/projects/{org}/{project}/keys/ returns multiple ProjectKey objects, the ProjectKeySerializer.serialize() method (line 104) calls obj.get_endpoint() for each key. Inside get_endpoint() (line 280), self.project.organization is accessed to check a feature flag. This access triggers a database query to load the organization if it's not already prefetched. Since the queryset used by ProjectKeysEndpoint.get() does not include select_related('project__organization'), this results in an N+1 query problem, where N is the number of ProjectKey objects. This issue is exacerbated as other properties like obj.csp_endpoint also call get_endpoint() internally, potentially leading to multiple organization lookups per key.
💡 Suggested Fix
Add select_related('project__organization') to the queryset in ProjectKeysEndpoint.get() or within the ProjectKey.objects.for_request() method to prefetch the organization.
🤖 Prompt for AI Agent
Fix this bug. In src/sentry/api/serializers/models/project_key.py at line 104: When the
API endpoint `/api/0/projects/{org}/{project}/keys/` returns multiple `ProjectKey`
objects, the `ProjectKeySerializer.serialize()` method (line 104) calls
`obj.get_endpoint()` for each key. Inside `get_endpoint()` (line 280),
`self.project.organization` is accessed to check a feature flag. This access triggers a
database query to load the organization if it's not already prefetched. Since the
queryset used by `ProjectKeysEndpoint.get()` does not include
`select_related('project__organization')`, this results in an N+1 query problem, where N
is the number of `ProjectKey` objects. This issue is exacerbated as other properties
like `obj.csp_endpoint` also call `get_endpoint()` internally, potentially leading to
multiple organization lookups per key.
Did we get this right? 👍 / 👎 to inform future reviews.
Alright final try at #102083 and #101999. We expose the integration endpoint in the project key serializer. API consumers can build the integrations (similar to how the `build_integration_endpoint` method works) for all the integration's that we'll be exposing in Relay. We might eventually remove the `otlp_traces_endpoint` and `otlp_logs_endpoint`, but we can explore that afterwards.
Alright final try at #102083 and #101999. We expose the integration endpoint in the project key serializer. API consumers can build the integrations (similar to how the `build_integration_endpoint` method works) for all the integration's that we'll be exposing in Relay. We might eventually remove the `otlp_traces_endpoint` and `otlp_logs_endpoint`, but we can explore that afterwards.
Alright final try at #102083 and #101999. We expose the integration endpoint in the project key serializer. API consumers can build the integrations (similar to how the `build_integration_endpoint` method works) for all the integration's that we'll be exposing in Relay. We might eventually remove the `otlp_traces_endpoint` and `otlp_logs_endpoint`, but we can explore that afterwards.
Alright final try at #102083 and #101999. We expose the integration endpoint in the project key serializer. API consumers can build the integrations (similar to how the `build_integration_endpoint` method works) for all the integration's that we'll be exposing in Relay. We might eventually remove the `otlp_traces_endpoint` and `otlp_logs_endpoint`, but we can explore that afterwards.
This is an alternative to #101999. By just exposing the endpoint (which is internally can be exposed with the
get_endpointmethod), we can construct the integration endpoints arbitrarily.